Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix: pass user-defined http(s) agent to websocket methods #953

Merged
merged 2 commits into from
Sep 19, 2019

Conversation

dpopp07
Copy link
Contributor

@dpopp07 dpopp07 commented Sep 13, 2019

We accept axios options in the constructor to give the user extended control over their requests but we didn't translate this control over to the WebSocket requests. This PR doesn't give complete control over the WebSocket configuration but it allows the use of custom "agents" which will allow users to use WebSockets behind corporate proxies (an established use case).

@codecov-io
Copy link

codecov-io commented Sep 13, 2019

Codecov Report

Merging #953 into master will decrease coverage by 14.28%.
The diff coverage is n/a.

Impacted file tree graph

@@             Coverage Diff             @@
##           master     #953       +/-   ##
===========================================
- Coverage   78.57%   64.28%   -14.29%     
===========================================
  Files           2        2               
  Lines          14       14               
  Branches        3        3               
===========================================
- Hits           11        9        -2     
- Misses          3        4        +1     
- Partials        0        1        +1
Impacted Files Coverage Δ
test/resources/service_error_util.js 20% <0%> (-60%) ⬇️
test/resources/auth_helper.js 88.88% <0%> (+11.11%) ⬆️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update e400240...2840d71. Read the comment docs.

@dpopp07 dpopp07 force-pushed the support-agent-websocket branch 2 times, most recently from efc4954 to 2617f4c Compare September 19, 2019 15:11
const socket = (this.socket = new w3cWebSocket(
url,
null,
null,
options.headers,
null,
requestOptions,
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yep, that's correct!

Copy link
Contributor

@germanattanasio germanattanasio left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I have used the axios agent option but never the websocket one. This looks good.

@dpopp07 dpopp07 merged commit 4f1679c into master Sep 19, 2019
@dpopp07 dpopp07 deleted the support-agent-websocket branch September 19, 2019 15:52
watson-github-bot pushed a commit that referenced this pull request Sep 19, 2019
## [4.5.1](v4.5.0...v4.5.1) (2019-09-19)

### Bug Fixes

* pass user-defined http(s) agent to websocket methods ([#953](#953)) ([4f1679c](4f1679c))
@watson-github-bot
Copy link
Member

🎉 This PR is included in version 4.5.1 🎉

The release is available on:

Your semantic-release bot 📦🚀

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Development

Successfully merging this pull request may close these issues.

None yet

4 participants